-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Performance: use IsSingleShard() check in pushDerived instead of just engine.EqualUnique opcode
#18974
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Performance: use IsSingleShard() check in pushDerived instead of just engine.EqualUnique opcode
#18974
Conversation
Signed-off-by: ChaitanyaD48 <[email protected]>
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
|
Can you also add test cases that show the impact of this change? |
Hi @arthurschreiber [
{
"comment": "Derived table pushdown optimization",
"query": "SELECT * FROM (SELECT DISTINCT col FROM user.ref WHERE col = 1 LIMIT 10) as sub",
"plan": {
"Type": "Passthrough",
"QueryType": "SELECT",
"Original": "SELECT * FROM (SELECT DISTINCT col FROM user.ref WHERE col = 1 LIMIT 10) as sub",
"Instructions": {
"OperatorType": "Route",
"Variant": "Reference",
"Keyspace": {
"Name": "user",
"Sharded": true
},
"FieldQuery": "select col from (select col from ref where 1 != 1) as sub where 1 != 1",
"Query": "select col from (select distinct col from ref where col = 1 limit 10) as sub"
},
"TablesUsed": [
"user.ref"
]
}
}
]Properties of the query -
These are the test results in both scenariosPrevious Case (EqualUnique-only):
Refactored Case (using IsSharded):
What do you think? |
|
Hi @arthurschreiber just following up on my previous comment. |
|
@ChaitanyaD48 The test case makes sense to me. 👍 |
…erived-use-isSingleShard Signed-off-by: ChaitanyaD48 <[email protected]>
Signed-off-by: ChaitanyaD48 <[email protected]>
|
Thanks! @arthurschreiber |
|
This PR is being marked as stale because it has been open for 30 days with no activity. To rectify, you may do any of the following:
If no action is taken within 7 days, this PR will be closed. |
…erived-use-isSingleShard
…erived-use-isSingleShard
Signed-off-by: ChaitanyaD48 <[email protected]>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #18974 +/- ##
==========================================
- Coverage 69.91% 69.90% -0.01%
==========================================
Files 1613 1613
Lines 216122 216124 +2
==========================================
- Hits 151097 151086 -11
- Misses 65025 65038 +13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|


Description
This PR enhances the
pushDerivedfunction inroute_planning.goto consider all single-shard routing scenarios, not just those using theengine.EqualUniqueopcode.IsSingleShard()method checks if a route is a single shard route by looking at its opcode. The method returnstruefor the following opcodes:Unsharded,DBA,Next,EqualUnique, andReference.Reference -
vitess/go/vt/vtgate/engine/routing.goRelated Issue(s)
Fixes #18952
Checklist
Deployment Notes
AI Disclosure